I'm not too happy about calling PrepareDNSServers() in deployFn(). @hellt opinion?

An option might be to do that stuff as part of https://github.com/srl-labs/containerlab/blob/main/types/topology.go#L508

We maybe add the ExtractDNSServerFromResolvConf() to the topology, cache the result and in the GetDNS function we evaluate if DNS servers are set, if not, we populate it with the determined system DNS IPs

I don't like this feature, as it causes labs to behave differently when deployed in different environments.

As a minimum it should be disabled by default, with an option/flag to enable it

@jbemmel what do you mean? Every environment will have its own DNS servers set up. If you need to rewrite it the default servers you do that manually.

As with regular docker we just let you leverage DNS servers you have in your env for srl nodes

@jbemmel what do you mean? Every environment will have its own DNS servers set up. If you need to rewrite it the default servers you do that manually.

As with regular docker we just let you leverage DNS servers you have in your env for srl nodes

My concern is that this feature adds non-reproducibility - the config you end up with in the nodes is different in a behind-the-scene context dependent way. That is a problem for certain usages of Containerlab (you'll have cases where things work in one lab, and then someone deploys the exact same lab in a different context and it doesn't work)

I'm saying the feature should be an explicit choice in the topology (it's like dhcp for dns server IPs)

This particular feature is perfectly in line with containerlab promises of delivering docker UX for networking labs.
With docker you have dns resolution that just works. Currently it doesn't for srl because of a different netns management lives in.

This particular feature is perfectly in line with containerlab promises of delivering docker UX for networking labs. With docker you have dns resolution that just works. Currently it doesn't for srl because of a different netns management lives in.

I agree it's a nice and useful feature, I'm merely saying that imho it should be explicit (or at least have the ability to disable it if as a user I want my lab to be self-contained, free from dependencies on the environment)

If you'd want to get rid of this you'd clear the DNS stanza with the startup config.

I don't see a reason why would you do that though, buy in 1% cases that would be the way to turn this feature off or rewrite it with a list of DNS servers you want to have.

I wouldn't want to introduce a flag or env var to turn this feature off until there is a legitimate use case behind it.

We will document the DNS provisioning in the srl doc

Codecov Report

Merging #1650 (01d5d8e) into main (f3d2553) will increase coverage by 0.12%.
Report is 5 commits behind head on main.
The diff coverage is 81.96%.

@@            Coverage Diff             @@
##             main    #1650      +/-   ##
==========================================
+ Coverage   50.88%   51.00%   +0.12%     
==========================================
  Files         135      136       +1     
  Lines       13107    13162      +55     
==========================================
+ Hits         6669     6713      +44     
- Misses       5706     5713       +7     
- Partials      732      736       +4